[AMD][AgentX] Tune MI300X Qwen3.5 MORI disaggregation with srt-slurm - #2984
[AMD][AgentX] Tune MI300X Qwen3.5 MORI disaggregation with srt-slurm#2984cquil11 wants to merge 4 commits into
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
a5b1a48 to
76d77d4
Compare
ee7b0e6 to
2369fb1
Compare
cquil11
changed the title
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34559632177 |
0513c80 to
252968b
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34565384525 |
|
Previous sweep reuse authorization withdrawn: the user requested adding TEP8/TEP8 points and running a new full official sweep. Run 34565384525 remains historical evidence for the earlier four-point matrix. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit eae1874. Configure here.
| if backend not in {"sglang", "dynamo"}: | ||
| raise SystemExit(f"Unsupported AIPERF_DRAIN_BACKEND: {backend}") | ||
| if not all(worker_urls): | ||
| raise SystemExit("Agentic drain requires non-empty worker metrics URLs") |
There was a problem hiding this comment.
Drain requires worker URLs unconditionally
Medium Severity
wait_for_agentic_servers_idle now treats an empty AIPERF_SERVER_METRICS_URLS as a fatal SystemExit for every backend. Existing Dynamo AgentX recipes that rely on this script never set that variable and previously drained from the frontend gauge alone. Because SystemExit bypasses the retry handler, any multi-concurrency job exits as soon as drain runs.
Reviewed by Cursor Bugbot for commit eae1874. Configure here.
|
/stage-results 34565384525 |
|
@cquil11 staged run 34565384525: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-09-11~r34565384525 This run remains available across future |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked utils/srt_slurm.py's image-cache handling: prepare_recipe only reads a pre-existing cached image path under SRT_SLURM_SHARED_BASE/containers and falls back to the raw image tag when no cached artifact exists — it never writes to or grows that directory itself, so this PR isn't introducing new unbounded cache growth outside GITHUB_WORKSPACE.
Extended reasoning...
This run reported findings (including a confirmed WORK_DIR-not-cleaned-up bug in runners/launch_srt_slurm.sh) that will be posted as inline comments, so approval is not appropriate. Beyond those, I independently checked utils/srt_slurm.py's prepare_recipe to see whether its image-cache lookup (image_cache / (image.replace(...) + '.sqsh'), backing --image-cache SRT_SLURM_SHARED_BASE/containers in runners/launch_srt_slurm.sh) itself creates or grows a persistent cache directory outside the CI workspace's clean lifecycle. It does not: the function only performs a read-only is_file() check and falls back to the plain image tag if no cached artifact is present, so this PR's own code path does not add to any cache-growth concern for that directory.
| SRT_SLURM_REPOSITORY="${SRT_SLURM_REPOSITORY:-https://github.com/SemiAnalysisAI/srt-slurm.git}" | ||
| SRT_SLURM_COMMIT="${SRT_SLURM_COMMIT:-81d46274f508e18ab14d1f123b75132005818dcf}" | ||
| RUN_KEY="${GITHUB_RUN_ID:-manual}-${GITHUB_RUN_ATTEMPT:-0}-${RUNNER_NAME:-runner}" | ||
| WORK_DIR=$(mktemp -d "${GITHUB_WORKSPACE}/.srt-slurm-${RUN_KEY}.XXXXXX") |
There was a problem hiding this comment.
🟡 (optional) WORK_DIR is created with mktemp under $GITHUB_WORKSPACE per invocation (unique per RUN_KEY) but is never removed on any exit path, including the success path and the cleanup() trap, unlike every other srt-slurm launcher in this repo (e.g. launch_b200-nscale-slurm.sh) which reuses a fixed dir and rm -rf's it before/after cloning specifically because GITHUB_WORKSPACE persists across jobs on these self-hosted runners. Each run leaves behind a full srt-slurm git clone plus its uv venv and outputs, so repeated CI runs (this is a full-sweep-enabled, repeatedly-scheduled workflow) accumulate GBs of clones/venvs on the shared runner disk indefinitely. Fix: rm -rf "$WORK_DIR" in cleanup() (or trap it separately) after collect_results has copied out what it needs, regardless of exit code.
Extended reasoning...
launch_mi300x-amd.sh execs launch_srt_slurm.sh whenever CONFIG_FILE is set (i.e. for every srt-slurm recipe run on this fleet). WORK_DIR=$(mktemp -d "$GITHUB_WORKSPACE/.srt-slurm-${RUN_KEY}.XXXXXX") creates a fresh directory named with GITHUB_RUN_ID-GITHUB_RUN_ATTEMPT-RUNNER_NAME, so it never collides with a prior run's directory (unlike the fixed 'srt-slurm' dir other launchers rm -rf before cloning into). git clone + make setup-compute + uv venv + uv pip install -e . populate SRT_REPO_DIR=$WORK_DIR/srt-slurm, and profile output_dir/results also live under $WORK_DIR. cleanup() (the EXIT trap) only calls the python adapter's collect (which copies files OUT of $WORK_DIR/results_root into $GITHUB_WORKSPACE) and then exit "$rc"; nothing in the file ever deletes $WORK_DIR itself. Because GITHUB_WORKSPACE is a persistent path on this self-hosted fleet (confirmed by sibling launchers explicitly rm -rf "$SRT_REPO_DIR" before every clone to avoid exactly this buildup), every successful or failed run of this new script leaves its full clone+venv on disk. Over the many scheduled…
Verification: nit. Factual claim is correct: launch_srt_slurm.sh:17 creates WORK_DIR=$(mktemp -d "${GITHUB_WORKSPACE}/.srt-slurm-${RUN_KEY}.XXXXXX") with a per-run-unique name (RUN_KEY=GITHUB_RUN_ID-RUN_ATTEMPT-RUNNER_NAME plus mktemp's .XXXXXX), and nothing in the script ever removes it. The cleanup() trap (lines 46-60) only scancels the job and runs the collect adapter; it explicitly does `trap - EXIT INT…
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34619026982 |
1 participant
Scope
Qwen3.5-397B-A17B FP8 AgentX disaggregation on MI300X using native MORI and srt-slurm. Based on main; no unrelated MI355X configurations or workflow changes.
lmsysorg/sglang-rocm:v0.5.19-rocm720-mi30x-20260910, digest pinned in each recipe.c6dc2f05061e504c59b8e7baa30cfeacabc93645, owns allocation, containers, router, workers, and benchmark lifecycle.Correctness blocker for requested mixed layout
TEP8 prefill to DEP8 decode changes attention TP8 to TP1. The pinned SGLang MORI transfer concatenates each local packed Q/K/V convolution-state shard instead of assembling each channel group separately. This silently rearranges Qwen recurrent state. The mixed layout is excluded pending a stock upstream fix.
The exact pinned transfer method was exercised on CPU with in-bounds byte-copy transport and actual model dimensions: 11,008 of 12,288 channels land incorrectly. Transfer implementation, grouped state definition.
Validation
no-evals: trueoption.full-sweep-enabledmatrix contains eight performance jobs, default warmup and 3600-second profiles, with no eval jobs. Fresh eight-point Run Sweep is in progress at7267a73c36d4a5d03f86a8bd79f32fbf0650ee16; prior sweep reuse authorization has been withdrawn.6da8c6f4695a57670b4ee8048373a94ce74933d1; all four performance jobs passed.agent/mi300x-qwen35-agentx-mori-frontier; the broad sweep was canceled at the user's request to narrow topology search.Previous four-point official performance results
All points use 16 physical GPUs. Interactivity below is P90 E2E-normalized output tokens/s/user; both displayed performance axes are higher-is-better. Values are from the official benchmark artifacts.
The DEP points are not performant: P90 time to first token is 328.26 seconds at c32 and 879.76 seconds at c64. They incurred 27 and 85 end-of-window cancellations respectively. The c64 artifact also records one
ClientPayloadErrorbefore mass cancellation, with an approximately 1800-second request lifetime, consistent with a stream timeout; the underlying reason for the long wait is not proven. The other three points record no client errors. CI success does not establish a performant Pareto frontier or model accuracy.